Skip to content

Add request-close command for dialogs #11045

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lukewarlow
Copy link
Member

@lukewarlow lukewarlow commented Feb 19, 2025

Add request-close command for dialogs
This new command maps to the dialog's requestClose function.

(See WHATWG Working Mode: Changes for more details.)


/form-elements.html ( diff )
/indices.html ( diff )
/interactive-elements.html ( diff )

@lukewarlow lukewarlow added the agenda+ To be discussed at a triage meeting label Feb 19, 2025
@past past removed the agenda+ To be discussed at a triage meeting label Feb 20, 2025
@domenic domenic added addition/proposal New features or enhancements topic: dialog The <dialog> element topic: close watchers labels Feb 21, 2025
@lukewarlow
Copy link
Member Author

I believe this is just waiting on implementor interest cc @smaug---- @annevk

@lukewarlow
Copy link
Member Author

@domenic can I take your comments during WhatNOT to be an official signal that Chrome is interested?

@domenic
Copy link
Member

domenic commented Feb 26, 2025

Thanks for checking! We should get @mfreed7 or @josepharhar to represent Chromium in that regard instead, as their team would be the one implementing it.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nit.

Regarding Chromium implementer interest, I see @mfreed7 already approved https://chromium-review.googlesource.com/c/chromium/src/+/6213975, so I think we can say they are interested.

@smaug---- was vaguely positive during WHATNOT but indeed it would be good to give him a chance to affirmatively comment on this specific PR text.

@lukewarlow lukewarlow added the agenda+ To be discussed at a triage meeting label Feb 26, 2025
@past past removed the agenda+ To be discussed at a triage meeting label Feb 27, 2025
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit concerned about a sprawling set of generic sounding commands with very specific actions, but I guess this is okay.

@lukewarlow
Copy link
Member Author

Is this ready to go, or given discussions in Slack is WebKit neutral rather than supportive on this?

@annevk
Copy link
Member

annevk commented Mar 3, 2025

You can consider WebKit as interested, but I think we should figure out #11092 as part of this if we think it'll impact the wording. Changing the HTML standard is very much a measure-twice-cut-once type of situation.

Or to put it another way: it's better that we figure this out than require everyone reading the standard to figure it out.

aarongable pushed a commit to chromium/chromium that referenced this pull request Mar 4, 2025
See whatwg/html#11045

Bug: 400647849
Change-Id: Iae7f2d431a097f3542fb3695f1746f8b513ca3c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6316649
Reviewed-by: Mason Freed <[email protected]>
Commit-Queue: Luke <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1428017}
lukewarlow added a commit to lukewarlow/WebKit that referenced this pull request Mar 11, 2025
https://bugs.webkit.org/show_bug.cgi?id=288476

Reviewed by NOBODY (OOPS!).

Spec PR: whatwg/html#11045

This patch implements the request-close command for dialog, this maps to the requestClose() method.

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/the-button-element/command-and-commandfor/on-dialog-behavior-request-close.tentative-expected.txt: Added.
* Source/WebCore/dom/Element.h:
* Source/WebCore/html/HTMLButtonElement.cpp:
(WebCore::HTMLButtonElement::commandType const):
* Source/WebCore/html/HTMLDialogElement.cpp:
(WebCore::HTMLDialogElement::isValidCommandType):
(WebCore::HTMLDialogElement::handleCommandInternal):
@lukewarlow lukewarlow marked this pull request as draft May 1, 2025 11:31
This new command maps to the dialog's requestClose function.

- Rename enable close watcher for requestClose()

- Add note to enable close watcher for request close
@lukewarlow lukewarlow force-pushed the request-close-command branch from 82c7bd0 to 3ffc0eb Compare May 22, 2025 14:26
@lukewarlow lukewarlow marked this pull request as ready for review May 22, 2025 14:26
@lukewarlow lukewarlow requested a review from annevk May 22, 2025 14:26
@lukewarlow
Copy link
Member Author

Apologies for squashing existing commits but it was a pain to rebase otherwise.

lukewarlow added a commit to lukewarlow/WebKit that referenced this pull request May 22, 2025
https://bugs.webkit.org/show_bug.cgi?id=288476

Reviewed by NOBODY (OOPS!).

Spec PR: whatwg/html#11045

This patch implements the request-close command for dialog, this maps to the requestClose() method.

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/html/semantics/the-button-element/command-and-commandfor/on-dialog-behavior-request-close.tentative-expected.txt: Added.
* Source/WebCore/dom/Element.h:
* Source/WebCore/html/HTMLButtonElement.cpp:
(WebCore::HTMLButtonElement::commandType const):
* Source/WebCore/html/HTMLDialogElement.cpp:
(WebCore::HTMLDialogElement::isValidCommandType):
(WebCore::HTMLDialogElement::handleCommandInternal):
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements topic: close watchers topic: dialog The <dialog> element
Development

Successfully merging this pull request may close these issues.

5 participants